feat: local auth bootstrapping - #7671

Beyond the three confirmed inline findings, I examined pkg/services/config.go around line 1069-1117 where bootstrapOptions = append(bootstrapOptions, bootstrap.Disabled()) unconditionally disables OBOT_BOOTSTRAP_TOKEN auth whenever the initial local-auth owner env vars (LocalAuthInitialOwnerEmail/LocalAuthInitialOwnerSetupToken) are configured, regardless of whether the owner has already been provisioned. This looked at first glance like it could permanently lock out bootstrap-token access, but reading the surrounding code confirms it is a deliberate design choice: local-auth owner provisioning and the legacy bootstrap-token mechanism are treated as mutually exclusive auth-bootstrapping paths, with a slog.Warn emitted to inform the operator when the token is being ignored. This is consistent with the ADR's described intent and is not a defect. The other candidate I looked at (missing spec for routes/activate/+page.svelte:61) duplicates one of the three findings already queued as inline comments, so it isn't new information. Given the security-sensitive nature of this change (auth bootstrapping, password-change flows, redirect handling) and the confirmed open-redirect and ordering bugs already flagged inline, this PR clearly warrants human review regardless of this additional check.

Suppressed comments (4)

pkg/api/handlers/authprovider.go:275

• The new staged settings are committed before the old verification cache is cleared. If this delete fails or the process stops here, the endpoint returns an error but the new settings remain staged; because activation matches proof only by provider name, the previous verification now authorizes activating settings that were never tested. Invalidate/version the proof atomically with replacing the staged credential (and account for in-flight verification callbacks).

	// These settings are not the ones any earlier verification ran against, so its result no longer
	// describes what activation would promote.
	if err := req.GatewayClient.ClearTempUserCache(req.Context()); err != nil {
		return fmt.Errorf("failed to clear the verification for the previous settings: %w", err)

pkg/api/handlers/localauth.go:174

• This cookie-authenticated password mutation has no CSRF/origin check. Context.Read unmarshals JSON regardless of Content-Type, so a same-site sibling origin can submit a crafted text/plain form with the victim's Lax cookie, choose an attacker-known password, and complete the owner setup. Require a same-origin request or enforce an equivalent CSRF boundary before accepting the body.

	var body localAuthUserRequest
	if err := req.Read(&body); err != nil {
		return types.NewErrBadRequest("invalid request body: %v", err)
	}

pkg/controller/handlers/providerconfigurationchange/providerconfigurationchange.go:258

• The replacement credential is deleted before the outgoing provider is deconfigured and before status/daemon updates complete. If any later operation fails, reconciliation retries without the staged credential (or sees the incoming provider as configured), converts the retry into a terminal error, and leaves the switch partially applied. Keep the operation's retry inputs until all steps finish and make the switched reconciliation explicitly idempotent for partially promoted/deconfigured states.

		// Cleared here rather than in the caller, so there is no window where a provider is both
		// configured and still staged against itself.
		if _, err := h.gatewayClient.DeleteCredential(ctx, system.ReplacementAuthProviderCredentialContext, authProvider.Name); err != nil {
			return fmt.Errorf("clear staged configuration for auth provider %q: %w", authProvider.Name, err)
		}
		if err := h.deconfigureAuthProvider(ctx, client, outgoingProvider); err != nil {

ui/user/src/routes/admin/auth-providers/+page.svelte:557

• This warning says everyone is signed out, but the verification session from the replacement provider is deliberately preserved so the user finishes the switch signed in. Describe the outgoing-provider sessions specifically to avoid misleading the owner about the post-switch state.

• Files reviewed: 67/68 changed files
• Comments generated: 3
• Review effort level: Balanced

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

pkg/api/handlers/setup/confirm_owner.go:68

• PromoteToOwner now creates a UserRoleChange itself, but ConfirmOwner still creates another one immediately below. A newly promoted bootstrap owner therefore emits two role-change resources and triggers duplicate propagation. Remove the old creation block from this handler.

pkg/api/handlers/authprovider.go:275

• Clearing only the cached result after staging does not invalidate an already-issued verification token. An OAuth callback for the previous settings can race this call (or arrive after the same provider is re-staged), repopulate TempSetupUser, and then satisfy Activate for credentials it never verified. Bind the token request/cache entry to the staged credential version and reject stale callbacks, or invalidate outstanding verification requests as part of the serialized staging change.

	// These settings are not the ones any earlier verification ran against, so its result no longer
	// describes what activation would promote.
	if err := req.GatewayClient.ClearTempUserCache(req.Context()); err != nil {
		return fmt.Errorf("failed to clear the verification for the previous settings: %w", err)

ui/user/src/routes/admin/auth-providers/+page.svelte:559

• This warning is not universally true. Verified providers are linked to an existing Obot user by verified email (pkg/gateway/client/identity.go:239-243), so a Google-to-GitHub switch using the same verified address reuses the user and their work. Make the copy conditional on provider identity semantics or explain that transfer depends on verified-email matching.

• Files reviewed: 67/68 changed files
• Comments generated: 2
• Review effort level: Balanced

Suppressed comments (6)

pkg/api/handlers/authprovider.go:271

• The new settings are committed before the old verification is cleared. If this cleanup fails transiently, the endpoint returns an error but the replacement settings remain staged and the old cache remains; Activate can then accept proof recorded for different credentials. Invalidate before committing, or bind the verification to a staged-credential revision so activation rejects stale proof.

	// These settings are not the ones any earlier verification ran against, so its result no longer
	// describes what activation would promote.
	if err := req.GatewayClient.ClearTempUserCache(req.Context()); err != nil {
		return fmt.Errorf("failed to clear the verification for the previous settings: %w", err)

pkg/api/handlers/authprovider.go:271

• Clearing only the cached result does not invalidate an OAuth verification already in flight. A callback started against settings A can arrive after this request stages settings B, repopulate the cache (the callback checks only the provider name), and then authorize activation of unverified settings B. Bind verification requests to the staged credential revision and reject/invalidate older requests when re-staging.

	// These settings are not the ones any earlier verification ran against, so its result no longer
	// describes what activation would promote.
	if err := req.GatewayClient.ClearTempUserCache(req.Context()); err != nil {
		return fmt.Errorf("failed to clear the verification for the previous settings: %w", err)

pkg/controller/handlers/providerconfigurationchange/providerconfigurationchange.go:255

• This switch is not retry-safe once deconfiguration starts. deconfigureAuthProvider deletes the outgoing credential before fallible session/table cleanup, and later status, daemon-sync, and staged-credential operations can also fail. A retry then sees the replacement as configured and line 227 records a terminal conflict, so activation can return failure after Local/current auth has already been disabled, contrary to the staged-switch failure guarantee. Make the switched reconcile idempotently resume after cutover or restore the outgoing credential on failure.

		if err := h.deconfigureAuthProvider(ctx, client, outgoingProvider); err != nil {
			return err
		}

pkg/gateway/server/oauth.go:155

• The verification is cached before owner promotion succeeds. If promotion is rejected (for example, for an explicitly configured Admin), the callback returns an error but leaves this cache entry behind; Activate checks only its provider name, so another Owner can activate a replacement whose verified identity never became Owner. Promote first and cache only after promotion succeeds.
  pkg/localauth/users.go:138
• Provisioning is latched only by the currently configured email. After owner A completes setup, changing the deployment email to B makes this lookup miss and the creation path below provisions a second pending initial owner. This contradicts the accepted design that changing owner settings after completion must not create another setup link. Persist or query a one-time provisioning marker independent of the mutable email before creating an initial owner.
  ui/user/src/lib/components/admin/McpServerEntryForm.svelte:737
• This catch turns a failed instance lookup into a successful-looking entry update. When existing deployments do exist, the failure prevents showUpdateExistingDeploymentsConfirm from being set, so users are never offered the propagation step and those deployments remain stale while the success message is shown below. Surface the lookup error or defer success until the deployment check can be retried.

• Files reviewed: 68/69 changed files
• Comments generated: 1
• Review effort level: Balanced

Suppressed comments (5)

pkg/api/handlers/authprovider.go:271

• Clearing only the completed cache does not invalidate OAuth verifications already in flight. If an owner starts verification and then re-stages this same provider before the callback returns, that callback still passes (the token is bound only to the provider name), repopulates the cache, and lets Activate promote credentials that were never used for the sign-in. Bind each verification to a staged credential revision/hash and reject or revoke it when staging changes.

	// These settings are not the ones any earlier verification ran against, so its result no longer
	// describes what activation would promote.
	if err := req.GatewayClient.ClearTempUserCache(req.Context()); err != nil {
		return fmt.Errorf("failed to clear the verification for the previous settings: %w", err)

pkg/controller/handlers/providerconfigurationchange/providerconfigurationchange.go:228

• This conflict check makes the switch non-idempotent after the incoming credential is promoted. If any later step fails after the outgoing credential is removed (status update, daemon revision, staged cleanup, etc.), the next reconcile sees the incoming provider as configured and terminally rejects its own partially applied change, leaving cleanup/status incomplete while the API reports failure. Treat configuredProvider == authProvider.Name as a resumable partial switch and make the remaining cleanup steps idempotent.

		if configuredProvider != change.Spec.ReplacesProviderName {
			return &authProviderConflictError{configuredProvider: configuredProvider}

pkg/gateway/server/oauth.go:156

• This durable Owner grant has no rollback when the switch is discarded, re-staged, or the user chooses “Sign in again”; those paths only clear TempSetupUser. Every identity used in an abandoned verification therefore remains Owner and can regain privileged access if this provider is staged or configured later. Preserve the cached prior role and restore it whenever verification is replaced/discarded, or use a restricted switch-completion capability instead of persisting Owner before activation.
  pkg/gateway/server/oauth.go:156
• The verification cache is committed before promotion succeeds. If promotion is rejected (for example, the identity is an environment-configured Admin) or fails, the callback returns an error but the cache still makes the provider appear verified, and another Owner can activate based on that stale row. Remove the cache on this failure path.
  pkg/localauth/users.go:142
• Provisioning is only made idempotent for the currently configured email. After the original owner completes setup, changing LOCAL_AUTH_INITIAL_OWNER_EMAIL makes this lookup miss and the function falls through to CreateInitialLocalAuthUser, creating a second activation link/account. That violates the accepted one-initial-owner behavior and the requirement that changing deployment settings after completion cannot create another setup link. Persist/check completion independently of the current email before creating a new pending owner.

• Files reviewed: 68/69 changed files
• Comments generated: 0 new
• Review effort level: Balanced

Suppressed comments (2)

pkg/api/handlers/authprovider.go:271

• Clear the previous verification before replacing the staged credential. As written, the controller has already persisted the new settings when ClearTempUserCache runs; if that cleanup fails, the request returns an error but the old cached verification remains valid for the newly staged settings, so Activate can promote settings that were never verified.

	// These settings are not the ones any earlier verification ran against, so its result no longer
	// describes what activation would promote.
	if err := req.GatewayClient.ClearTempUserCache(req.Context()); err != nil {
		return fmt.Errorf("failed to clear the verification for the previous settings: %w", err)

pkg/controller/handlers/providerconfigurationchange/providerconfigurationchange.go:228

• This precondition makes the switch non-idempotent after partial success. Once the incoming credential is promoted and deconfigureAuthProvider deletes the outgoing credential, any later failure (session cleanup, status update, daemon revision, or staged-credential deletion) causes the retry to observe the incoming provider as configured and record a conflict here. The remaining cleanup can then never finish, even though activation reports failure and Local may already be disabled. Model the switch as resumable phases (or otherwise recognize the already-promoted replacement) so reconciliation can complete after failures.

		if configuredProvider != change.Spec.ReplacesProviderName {
			return &authProviderConflictError{configuredProvider: configuredProvider}

• Files reviewed: 68/69 changed files
• Comments generated: 2
• Review effort level: Balanced

Suppressed comments (3)

pkg/api/handlers/authprovider.go:271

• Clearing only the cached result does not retire verification token requests that were issued for the previous settings. An in-flight old OAuth flow can finish after this clear, repopulate the cache for the same provider name, and make Activate promote the newly staged credentials even though the successful login used the prior configuration. Bind verification requests to a staged-credential generation and invalidate/check that generation atomically when settings are replaced.

	// These settings are not the ones any earlier verification ran against, so its result no longer
	// describes what activation would promote.
	if err := req.GatewayClient.ClearTempUserCache(req.Context()); err != nil {
		return fmt.Errorf("failed to clear the verification for the previous settings: %w", err)

pkg/controller/handlers/providerconfigurationchange/providerconfigurationchange.go:228

• Make the switch retry-safe. Once the replacement credential is promoted and the outgoing credential is removed below, any transient failure in the status, daemon-revision, or staged-credential cleanup steps causes the next reconcile to see the replacement as configuredProvider. This check then records a terminal conflict, so reconciliation cannot finish and the API can report failure even though the login provider already changed. Treat the already-promoted state as resumable (or persist explicit phases) and continue the remaining cleanup.

		if configuredProvider != change.Spec.ReplacesProviderName {
			return &authProviderConflictError{configuredProvider: configuredProvider}

pkg/localauth/users.go:138

• The idempotency check is scoped only to the currently configured email. After the first owner completes setup, changing the deployment email makes this lookup miss and the code below creates a second pending “initial” owner and setup link. The accepted design explicitly excludes creating more than one initial owner and states that changing the owner email after completion must not create a new setup link. Persist and check installation-wide completion state independently of the current email before creating another pending owner.

• Files reviewed: 68/69 changed files
• Comments generated: 2
• Review effort level: Balanced